Skip to content

Let stub-defined __all__ override imports #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Let stub-defined __all__ override imports #133

wants to merge 1 commit into from
+73 −1

Conversation

@lagru
Copy link
Member

@lagru lagru commented Nov 27, 2024 •
edited by stefanv
Loading

In scikit-image, I encountered the case where I want to make certain functions available for lazy-loading while at the same time not wanting to advertise their existence via __all__ (see scikit-image/scikit-image#6892). To implement this with the current behavior of lazy_loader.attach_stub I had to specify __all__ twice, both in the PY and in the PYI file (see scikit-image/scikit-image@40157d0).

Ideally, I'd like to avoid this duplication. So I came up with a quick "work in progress" solution, in order to discuss this use case here. The current draft has some drawbacks for now, e.g. it makes certain assumptions about how __all__ must be defined in the stub. I'm happy to iterate on this, depending on which kind of behavior is wanted.

Please, let me know what you think. 😊

@codecov
Copy link

codecov bot commented Nov 27, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 78.78788% with 7 lines in your changes missing coverage. Please review.

Project coverage is 93.63%. Comparing base (4e78314) to head (9579004).

Files with missing lines Patch % Lines
lazy_loader/__init__.py 70.83% 7 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
- Coverage   95.74%   93.63%   -2.12%     
==========================================
  Files           5        5              
  Lines         235      267      +32     
==========================================
+ Hits          225      250      +25     
- Misses         10       17       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lagru
Let stub-defined __all__ override imports
9579004 
This enables the use case where objects should be available for
lazy-loading while not advertising them. This might be useful for
deprecations.
@lagru lagru force-pushed the include_stub__all__ branch from 464728f to 9579004 Compare November 27, 2024 14:06
@lagru lagru mentioned this pull request Nov 27, 2024
Merged
@stefanv
Copy link
Member

stefanv commented Dec 5, 2024

__all__ is really only used when doing from X import * isn't it? So, mostly I think __dir__ is what we care about. Then there are two options: populate __dir__ as everything that is lazy loaded, or take into consideration __all__.

Could that be a simpler approach, here, simply adding a use_all flag that changes both what comes back in __all__ and in __dir__?

@lagru
Copy link
Member Author

lagru commented Dec 6, 2024

all is really only used when doing from X import * isn't it?

Probably? There might be tools out there to find the public API that way. Can't remember how Sphinx does it, but probably with dir()?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lagru @stefanv